Optimize const grouping keys#17642
Conversation
kaikalur
left a comment
There was a problem hiding this comment.
Also squash the commits
presto-main/src/main/java/com/facebook/presto/SystemSessionProperties.java
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
thanks, will do, copy-paste error
There was a problem hiding this comment.
I believe we have a isConst method in ExpressionUtils?
There was a problem hiding this comment.
done: the utility function expects Expression, I added a wrapper below
There was a problem hiding this comment.
@kaikalur is there a way to check if a variable reference is a constant without hard-coding the type of child node? that would miss cases when the constant is introduced further down in the tree, or is the result of a filter. Not sure if we support that type of property/constraint derivation?
There was a problem hiding this comment.
We are always rewriting the plans through all the optimizations so maybe just doing this at the end (just before exchanges) will do it. And I think this is the way to capture the children.
There was a problem hiding this comment.
I think the way to do that would be to have additional "pull up constant" rules for other node types. We can run all of them until the constant is as far up in the plan as it goes.
138a040 to
a0f280f
Compare
There was a problem hiding this comment.
You might want to add the expected one as well - for example:
select 'blah', * from (select regiokey, count(*) from nation group by regionkey)
So it is more obvious.
There was a problem hiding this comment.
static import isConstant
Our style is to use static imports unless it would not be obvious without the fully qualified name
There was a problem hiding this comment.
I think the way to do that would be to have additional "pull up constant" rules for other node types. We can run all of them until the constant is as far up in the plan as it goes.
There was a problem hiding this comment.
instead override the method "isEnabled"
There was a problem hiding this comment.
this doesn't need a special variable since it only adds one rule
There was a problem hiding this comment.
I would actually tag this on to some other block of rules. It doesn't need to be off by itself. Also, make sure inlineProjections gets called after this (InlineProjections and RemoveRedundantIdentityProject).
There was a problem hiding this comment.
this spacing isn't correct. the arguments here should be indented another level
There was a problem hiding this comment.
static import Function.identity()
There was a problem hiding this comment.
In the pattern you also need to check that the aggregation only has a single grouping set. Then you should add a test that shows the optimization is not applied if there is more than one grouping set
There was a problem hiding this comment.
done + added a new test
There was a problem hiding this comment.
all names should be camel case so testNotApplicable(). Also, add a more descriptive name (even if it's kind of long), as we want to also test other not applicable cases, and it should be obvious from looking at the test name exactly what is being tested e.g. all the not applicable tests could be testXXXDoesNotFier()
There was a problem hiding this comment.
Some additional cases to test
- does not fire with the flag disabled
- multiple constants
- if only some constants aren't in the aggregation output, they don't get projected, but other constants do
- Multiple grouping sets
There was a problem hiding this comment.
added 1,2&4
having issues with plan matching for # 3 (let's discuss offline)
9fac2a4 to
82ec363
Compare
There was a problem hiding this comment.
Since we use source of an aggregation, I think it will be good to use that same terminology here instead of child so it's easier to understand.
There was a problem hiding this comment.
And you should rewrite the child to drop the constant assignment and outputs
There was a problem hiding this comment.
this is not always correct, the constant variable may be used as an argument to one of the agg functions, for example
select avg(x) from (select 1 as x) group by x;
I think pruning unused columns should be done separately as it may show up in other cases as well
There was a problem hiding this comment.
You can call PruneUnreferencedOutputs after this optimizer runs
rschlussel
left a comment
There was a problem hiding this comment.
minor comments, but looks great
There was a problem hiding this comment.
give this a descriptive name e.g. testSingleConstColumn
There was a problem hiding this comment.
static import expression
d060d0b to
612ae03
Compare
|
@mlyublena you still need to sign the CLA |
|
Sorry, noticed the commit message title is too long and I don't think I have permission to update your branch myself. We follow https://cbea.ms/git-commit/ for our commit guidelines. Can you change the commit title to "Move constant grouping keys above the group by" and put "For example" on the next line? |
612ae03 to
f02c07e
Compare
yingsu00
left a comment
There was a problem hiding this comment.
@mlyublena Could you please check if the query SELECT userid, 'constant val' as extra, cnt from (SELECT userid, AS extra, COUNT(1) from T GROUP BY 1) s in the commit message is correct? Did you mean this?
SELECT userid, 'constant val' as extra, cnt from (SELECT userid, COUNT(1) as cnt from t GROUP BY 1) s;
There was a problem hiding this comment.
Added a benchmark test @kaikalur:
INFO: Without optimization
Nov 04, 2022 11:13:36 AM com.facebook.airlift.log.Logger info
INFO: -- Loading system access control --
Nov 04, 2022 11:13:36 AM com.facebook.airlift.log.Logger info
INFO: -- Loaded system access control allow-all --
Nov 04, 2022 11:13:36 AM com.facebook.airlift.log.Logger info
INFO: -- Loading temp storage local --
Nov 04, 2022 11:13:36 AM com.facebook.airlift.log.Logger info
INFO: -- Loaded temp storage local --
Nov 04, 2022 11:13:36 AM com.facebook.airlift.log.Logger info
INFO: -- Loading temp storage local --
Nov 04, 2022 11:13:36 AM com.facebook.airlift.log.Logger info
INFO: -- Loaded temp storage local --
peak_memory:1049052,elapsed_millis:71,input_rows_per_second:352,output_rows_per_second:70,input_megabytes:0,input_megabytes_per_second:0,wall_nanos:70884750,cpu_nanos:63690000,user_nanos:59689000,input_rows:25,input_bytes:0,output_rows:5,output_bytes:130
peak_memory:1049052,elapsed_millis:88,input_rows_per_second:282,output_rows_per_second:56,input_megabytes:0,input_megabytes_per_second:0,wall_nanos:88427792,cpu_nanos:82240000,user_nanos:76153000,input_rows:25,input_bytes:0,output_rows:5,output_bytes:130
peak_memory:1049052,elapsed_millis:73,input_rows_per_second:342,output_rows_per_second:68,input_megabytes:0,input_megabytes_per_second:0,wall_nanos:73053500,cpu_nanos:66569000,user_nanos:61794000,input_rows:25,input_bytes:0,output_rows:5,output_bytes:130
peak_memory:1049052,elapsed_millis:90,input_rows_per_second:277,output_rows_per_second:55,input_megabytes:0,input_megabytes_per_second:0,wall_nanos:89986583,cpu_nanos:83379000,user_nanos:76933000,input_rows:25,input_bytes:0,output_rows:5,output_bytes:130
peak_memory:1049052,elapsed_millis:81,input_rows_per_second:307,output_rows_per_second:61,input_megabytes:0,input_megabytes_per_second:0,wall_nanos:81414083,cpu_nanos:72700000,user_nanos:68129000,input_rows:25,input_bytes:0,output_rows:5,output_bytes:130
peak_memory:1049052,elapsed_millis:75,input_rows_per_second:335,output_rows_per_second:67,input_megabytes:0,input_megabytes_per_second:0,wall_nanos:74596792,cpu_nanos:70166000,user_nanos:65303000,input_rows:25,input_bytes:0,output_rows:5,output_bytes:130
peak_memory:1049052,elapsed_millis:73,input_rows_per_second:341,output_rows_per_second:68,input_megabytes:0,input_megabytes_per_second:0,wall_nanos:73201583,cpu_nanos:67967000,user_nanos:62847000,input_rows:25,input_bytes:0,output_rows:5,output_bytes:130
peak_memory:1049052,elapsed_millis:96,input_rows_per_second:261,output_rows_per_second:52,input_megabytes:0,input_megabytes_per_second:0,wall_nanos:95575666,cpu_nanos:85599000,user_nanos:78474000,input_rows:25,input_bytes:0,output_rows:5,output_bytes:130
peak_memory:1049052,elapsed_millis:151,input_rows_per_second:165,output_rows_per_second:33,input_megabytes:0,input_megabytes_per_second:0,wall_nanos:151015041,cpu_nanos:135654000,user_nanos:123756000,input_rows:25,input_bytes:0,output_rows:5,output_bytes:130
peak_memory:1049052,elapsed_millis:153,input_rows_per_second:163,output_rows_per_second:32,input_megabytes:0,input_megabytes_per_second:0,wall_nanos:153300500,cpu_nanos:124190000,user_nanos:114321000,input_rows:25,input_bytes:0,output_rows:5,output_bytes:130
peak_memory:1049052,elapsed_millis:150,input_rows_per_second:166,output_rows_per_second:33,input_megabytes:0,input_megabytes_per_second:0,wall_nanos:149765583,cpu_nanos:139011000,user_nanos:127773000,input_rows:25,input_bytes:0,output_rows:5,output_bytes:130
peak_memory:1049052,elapsed_millis:121,input_rows_per_second:206,output_rows_per_second:41,input_megabytes:0,input_megabytes_per_second:0,wall_nanos:121312292,cpu_nanos:113491000,user_nanos:105332000,input_rows:25,input_bytes:0,output_rows:5,output_bytes:130
peak_memory:1049052,elapsed_millis:58,input_rows_per_second:428,output_rows_per_second:85,input_megabytes:0,input_megabytes_per_second:0,wall_nanos:58363583,cpu_nanos:53699000,user_nanos:50201000,input_rows:25,input_bytes:0,output_rows:5,output_bytes:130
peak_memory:1049052,elapsed_millis:67,input_rows_per_second:371,output_rows_per_second:74,input_megabytes:0,input_megabytes_per_second:0,wall_nanos:67276167,cpu_nanos:62683000,user_nanos:58266000,input_rows:25,input_bytes:0,output_rows:5,output_bytes:130
peak_memory:1049052,elapsed_millis:59,input_rows_per_second:421,output_rows_per_second:84,input_megabytes:0,input_megabytes_per_second:0,wall_nanos:59347209,cpu_nanos:55699000,user_nanos:51709000,input_rows:25,input_bytes:0,output_rows:5,output_bytes:130
peak_memory:1049052,elapsed_millis:51,input_rows_per_second:493,output_rows_per_second:98,input_megabytes:0,input_megabytes_per_second:0,wall_nanos:50684583,cpu_nanos:47907000,user_nanos:45072000,input_rows:25,input_bytes:0,output_rows:5,output_bytes:130
peak_memory:1049052,elapsed_millis:53,input_rows_per_second:475,output_rows_per_second:95,input_megabytes:0,input_megabytes_per_second:0,wall_nanos:52540792,cpu_nanos:48898000,user_nanos:45745000,input_rows:25,input_bytes:0,output_rows:5,output_bytes:130
peak_memory:1049052,elapsed_millis:77,input_rows_per_second:326,output_rows_per_second:65,input_megabytes:0,input_megabytes_per_second:0,wall_nanos:76682500,cpu_nanos:69061000,user_nanos:63892000,input_rows:25,input_bytes:0,output_rows:5,output_bytes:130
peak_memory:1049052,elapsed_millis:57,input_rows_per_second:441,output_rows_per_second:88,input_megabytes:0,input_megabytes_per_second:0,wall_nanos:56605834,cpu_nanos:52881000,user_nanos:49281000,input_rows:25,input_bytes:0,output_rows:5,output_bytes:130
peak_memory:1049052,elapsed_millis:54,input_rows_per_second:459,output_rows_per_second:91,input_megabytes:0,input_megabytes_per_second:0,wall_nanos:54454666,cpu_nanos:51225000,user_nanos:47822000,input_rows:25,input_bytes:0,output_rows:5,output_bytes:130
pull_constants_above_group_by :: 77.335 cpu ms :: 1MB peak memory :: in 25, 0B, 323/s, 0B/s :: out 5, 130B, 64/s, 1.64KB/s
Nov 04, 2022 11:13:45 AM com.facebook.airlift.log.Logger info
INFO: With optimization
Nov 04, 2022 11:13:45 AM com.facebook.airlift.log.Logger info
INFO: -- Loading system access control --
Nov 04, 2022 11:13:45 AM com.facebook.airlift.log.Logger info
INFO: -- Loaded system access control allow-all --
Nov 04, 2022 11:13:45 AM com.facebook.airlift.log.Logger info
INFO: -- Loading temp storage local --
Nov 04, 2022 11:13:45 AM com.facebook.airlift.log.Logger info
INFO: -- Loaded temp storage local --
Nov 04, 2022 11:13:45 AM com.facebook.airlift.log.Logger info
INFO: -- Loading temp storage local --
Nov 04, 2022 11:13:45 AM com.facebook.airlift.log.Logger info
INFO: -- Loaded temp storage local --
peak_memory:1495616,elapsed_millis:37,input_rows_per_second:684,output_rows_per_second:136,input_megabytes:0,input_megabytes_per_second:0,wall_nanos:36547500,cpu_nanos:35853000,user_nanos:30955000,input_rows:25,input_bytes:0,output_rows:5,output_bytes:130
peak_memory:1495616,elapsed_millis:42,input_rows_per_second:595,output_rows_per_second:119,input_megabytes:0,input_megabytes_per_second:0,wall_nanos:42003750,cpu_nanos:40892000,user_nanos:34889000,input_rows:25,input_bytes:0,output_rows:5,output_bytes:130
peak_memory:1495616,elapsed_millis:59,input_rows_per_second:421,output_rows_per_second:84,input_megabytes:0,input_megabytes_per_second:0,wall_nanos:59294625,cpu_nanos:55689000,user_nanos:47761000,input_rows:25,input_bytes:0,output_rows:5,output_bytes:130
peak_memory:1495616,elapsed_millis:48,input_rows_per_second:522,output_rows_per_second:104,input_megabytes:0,input_megabytes_per_second:0,wall_nanos:47834833,cpu_nanos:45860000,user_nanos:39594000,input_rows:25,input_bytes:0,output_rows:5,output_bytes:130
peak_memory:1495616,elapsed_millis:36,input_rows_per_second:693,output_rows_per_second:138,input_megabytes:0,input_megabytes_per_second:0,wall_nanos:36051750,cpu_nanos:35401000,user_nanos:30276000,input_rows:25,input_bytes:0,output_rows:5,output_bytes:130
peak_memory:1495616,elapsed_millis:108,input_rows_per_second:232,output_rows_per_second:46,input_megabytes:0,input_megabytes_per_second:0,wall_nanos:107698834,cpu_nanos:67399000,user_nanos:60120000,input_rows:25,input_bytes:0,output_rows:5,output_bytes:130
peak_memory:1495616,elapsed_millis:89,input_rows_per_second:279,output_rows_per_second:55,input_megabytes:0,input_megabytes_per_second:0,wall_nanos:89431750,cpu_nanos:87399000,user_nanos:82486000,input_rows:25,input_bytes:0,output_rows:5,output_bytes:130
peak_memory:1495616,elapsed_millis:36,input_rows_per_second:702,output_rows_per_second:140,input_megabytes:0,input_megabytes_per_second:0,wall_nanos:35595250,cpu_nanos:34275000,user_nanos:32643000,input_rows:25,input_bytes:0,output_rows:5,output_bytes:130
peak_memory:1495616,elapsed_millis:39,input_rows_per_second:642,output_rows_per_second:128,input_megabytes:0,input_megabytes_per_second:0,wall_nanos:38891708,cpu_nanos:37020000,user_nanos:34996000,input_rows:25,input_bytes:0,output_rows:5,output_bytes:130
peak_memory:1495616,elapsed_millis:34,input_rows_per_second:732,output_rows_per_second:146,input_megabytes:0,input_megabytes_per_second:0,wall_nanos:34141125,cpu_nanos:32628000,user_nanos:30786000,input_rows:25,input_bytes:0,output_rows:5,output_bytes:130
peak_memory:1495616,elapsed_millis:38,input_rows_per_second:653,output_rows_per_second:130,input_megabytes:0,input_megabytes_per_second:0,wall_nanos:38242459,cpu_nanos:36824000,user_nanos:35107000,input_rows:25,input_bytes:0,output_rows:5,output_bytes:130
peak_memory:1495616,elapsed_millis:48,input_rows_per_second:520,output_rows_per_second:104,input_megabytes:0,input_megabytes_per_second:0,wall_nanos:48046416,cpu_nanos:44795000,user_nanos:41783000,input_rows:25,input_bytes:0,output_rows:5,output_bytes:130
peak_memory:1495616,elapsed_millis:38,input_rows_per_second:664,output_rows_per_second:132,input_megabytes:0,input_megabytes_per_second:0,wall_nanos:37624625,cpu_nanos:36067000,user_nanos:34233000,input_rows:25,input_bytes:0,output_rows:5,output_bytes:130
peak_memory:1495616,elapsed_millis:29,input_rows_per_second:860,output_rows_per_second:172,input_megabytes:0,input_megabytes_per_second:0,wall_nanos:29043125,cpu_nanos:28326000,user_nanos:27263000,input_rows:25,input_bytes:0,output_rows:5,output_bytes:130
peak_memory:1495616,elapsed_millis:53,input_rows_per_second:470,output_rows_per_second:94,input_megabytes:0,input_megabytes_per_second:0,wall_nanos:53106041,cpu_nanos:49474000,user_nanos:46099000,input_rows:25,input_bytes:0,output_rows:5,output_bytes:130
peak_memory:1495616,elapsed_millis:45,input_rows_per_second:553,output_rows_per_second:110,input_megabytes:0,input_megabytes_per_second:0,wall_nanos:45162917,cpu_nanos:42568000,user_nanos:39742000,input_rows:25,input_bytes:0,output_rows:5,output_bytes:130
peak_memory:1495616,elapsed_millis:40,input_rows_per_second:619,output_rows_per_second:123,input_megabytes:0,input_megabytes_per_second:0,wall_nanos:40335667,cpu_nanos:36132000,user_nanos:34354000,input_rows:25,input_bytes:0,output_rows:5,output_bytes:130
peak_memory:1495616,elapsed_millis:43,input_rows_per_second:581,output_rows_per_second:116,input_megabytes:0,input_megabytes_per_second:0,wall_nanos:43028208,cpu_nanos:40935000,user_nanos:38190000,input_rows:25,input_bytes:0,output_rows:5,output_bytes:130
peak_memory:1495616,elapsed_millis:30,input_rows_per_second:822,output_rows_per_second:164,input_megabytes:0,input_megabytes_per_second:0,wall_nanos:30398000,cpu_nanos:29613000,user_nanos:28504000,input_rows:25,input_bytes:0,output_rows:5,output_bytes:130
peak_memory:1495616,elapsed_millis:29,input_rows_per_second:867,output_rows_per_second:173,input_megabytes:0,input_megabytes_per_second:0,wall_nanos:28825333,cpu_nanos:28177000,user_nanos:27255000,input_rows:25,input_bytes:0,output_rows:5,output_bytes:130
pull_constants_above_group_by :: 42.266 cpu ms :: 1.43MB peak memory :: in 25, 0B, 591/s, 0B/s :: out 5, 130B, 118/s, 3KB/s
|
Please squash the commits into a single one |
For example: SELECT userid, 'constant val' AS extra, COUNT(1) as cnt from T GROUP BY 1, 2 into SELECT userid, 'constant val' as extra, cnt from (SELECT userid, AS extra, COUNT(1) from T GROUP BY 1) s
3cd42fe to
00b69cc
Compare
Move constant grouping keys into a Project node above the group by, for example:
into
Test plan